Skip to content

Remove AlignedBoxWithSlice wrapper and add alias to Poly<[T], AlignedAllocator>#965

Merged
JordanMaples merged 4 commits intomainfrom
jordanmaples/replace_AlignedBoxWithSlice
Apr 22, 2026
Merged

Remove AlignedBoxWithSlice wrapper and add alias to Poly<[T], AlignedAllocator>#965
JordanMaples merged 4 commits intomainfrom
jordanmaples/replace_AlignedBoxWithSlice

Conversation

@JordanMaples
Copy link
Copy Markdown
Contributor

Closes #907

This change introduces AlignedSlice<T> type alias and the aligned_slice() constructor in diskann-quantization::alloc, replacing the AlignedBoxWithSlice wrapper struct from diskann-providers. This eliminates an unnecessary abstraction layer and moves aligned allocation to its proper tier.

  • Add AlignedSlice<T> = Poly<[T], AlignedAllocator> type alias
  • Add aligned_slice<T>(capacity, alignment) constructor
  • Migrate all 9 consumer files in diskann-disk
  • Replace split_into_nonoverlapping_mut_slices with chunks_mut
  • Replace as_slice/as_mut_slice with Deref/DerefMut
  • Delete aligned_allocator.rs from diskann-providers
  • Add 5 tests for aligned_slice

…907)

Introduce AlignedSlice<T> type alias and aligned_slice() constructor in
diskann-quantization::alloc, replacing the AlignedBoxWithSlice wrapper
struct from diskann-providers. This eliminates an unnecessary abstraction
layer and moves aligned allocation to its proper tier.

- Add AlignedSlice<T> = Poly<[T], AlignedAllocator> type alias
- Add aligned_slice<T>(capacity, alignment) constructor
- Migrate all 9 consumer files in diskann-disk
- Replace split_into_nonoverlapping_mut_slices with chunks_mut
- Replace as_slice/as_mut_slice with Deref/DerefMut
- Delete aligned_allocator.rs from diskann-providers
- Add 5 tests for aligned_slice

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JordanMaples JordanMaples requested review from a team and Copilot April 21, 2026 20:29
@JordanMaples
Copy link
Copy Markdown
Contributor Author

This replaces #908

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the AlignedBoxWithSlice wrapper from diskann-providers and standardizes aligned slice allocation in diskann-quantization::alloc via a new AlignedSlice<T> alias and aligned_slice() constructor, updating disk-index readers/search code to use the new API.

Changes:

  • Add AlignedSlice<T> = Poly<[T], AlignedAllocator> and aligned_slice<T>(capacity, alignment: PowerOfTwo) in diskann-quantization.
  • Migrate aligned buffer usage across diskann-disk (tests/benches/search) to the new constructor and to slice deref semantics.
  • Remove diskann-providers::common::AlignedBoxWithSlice and replace custom splitting with chunks_mut.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
diskann-quantization/src/alloc/mod.rs Exposes new aligned-slice module + re-exports AlignedSlice/aligned_slice.
diskann-quantization/src/alloc/aligned_slice.rs Introduces AlignedSlice<T> alias, constructor, and unit tests.
diskann-providers/src/common/mod.rs Drops AlignedBoxWithSlice re-export.
diskann-providers/src/common/aligned_allocator.rs Deletes the wrapper implementation (and its tests).
diskann-disk/src/utils/aligned_file_reader/windows_aligned_file_reader.rs Updates tests to allocate aligned buffers via aligned_slice and split via chunks_mut.
diskann-disk/src/utils/aligned_file_reader/storage_provider_aligned_file_reader.rs Same migration as above for storage-provider reader tests.
diskann-disk/src/utils/aligned_file_reader/linux_aligned_file_reader.rs Same migration as above for Linux reader tests.
diskann-disk/src/utils/aligned_file_reader/aligned_read.rs Updates/shortens doc comment reflecting removal of wrapper type.
diskann-disk/src/search/provider/disk_vertex_provider_factory.rs Uses aligned_slice buffer for header reads and relies on deref slicing.
diskann-disk/src/search/provider/disk_sector_graph.rs Switches internal storage to AlignedSlice<u8> and replaces custom splitting with chunks_mut.
diskann-disk/src/search/provider/disk_provider.rs Removes .as_slice() calls; relies on deref to slice for .to_vec().
diskann-disk/src/search/pq/quantizer_preprocess.rs Replaces .as_mut_slice() with deref-based mutable slicing/borrows.
diskann-disk/src/search/pq/pq_scratch.rs Replaces wrapper fields with AlignedSlice allocations and updates alignment tests.
diskann-disk/benches/benchmarks_iai/aligned_file_reader_bench_iai.rs Updates benchmark to use aligned_slice + chunks_mut.
diskann-disk/benches/benchmarks/aligned_file_reader_bench.rs Updates benchmark to use aligned_slice + chunks_mut.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread diskann-quantization/src/alloc/aligned_slice.rs Outdated
Comment thread diskann-disk/src/search/provider/disk_sector_graph.rs
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 91.76471% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.31%. Comparing base (f23d38b) to head (51104f2).

Files with missing lines Patch % Lines
...kann-disk/src/search/provider/disk_sector_graph.rs 71.42% 6 Missing ⚠️
diskann-disk/src/search/pq/quantizer_preprocess.rs 66.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #965      +/-   ##
==========================================
- Coverage   90.47%   89.31%   -1.17%     
==========================================
  Files         448      448              
  Lines       83447    83329     -118     
==========================================
- Hits        75497    74422    -1075     
- Misses       7950     8907     +957     
Flag Coverage Δ
miri 89.31% <91.76%> (-1.17%) ⬇️
unittests 89.15% <91.76%> (-1.29%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
diskann-disk/src/search/pq/pq_scratch.rs 79.54% <100.00%> (+0.97%) ⬆️
diskann-disk/src/search/provider/disk_provider.rs 90.89% <ø> (-0.01%) ⬇️
...rc/search/provider/disk_vertex_provider_factory.rs 95.70% <100.00%> (+0.05%) ⬆️
...disk/src/utils/aligned_file_reader/aligned_read.rs 100.00% <ø> (ø)
...s/aligned_file_reader/linux_aligned_file_reader.rs 82.65% <100.00%> (-0.35%) ⬇️
...ile_reader/storage_provider_aligned_file_reader.rs 98.36% <100.00%> (-0.03%) ⬇️
diskann-quantization/src/alloc/aligned_slice.rs 100.00% <100.00%> (ø)
diskann-quantization/src/alloc/mod.rs 100.00% <ø> (ø)
diskann-disk/src/search/pq/quantizer_preprocess.rs 88.00% <66.66%> (ø)
...kann-disk/src/search/provider/disk_sector_graph.rs 96.94% <71.42%> (-1.24%) ⬇️

... and 39 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JordanMaples and others added 2 commits April 21, 2026 13:47
Poly::from_iter special-cases len == 0 and returns an empty slice
rather than an AllocatorError. Update the test to assert Ok + empty
slice instead of discarding the result.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Return ANNError instead of panicking on division by zero if
num_sectors_per_node or block_size is zero. The constructor currently
prevents this, but this guards against corrupt metadata defensively.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JordanMaples JordanMaples enabled auto-merge (squash) April 21, 2026 20:53
@JordanMaples JordanMaples merged commit a0dfde6 into main Apr 22, 2026
26 checks passed
@JordanMaples JordanMaples deleted the jordanmaples/replace_AlignedBoxWithSlice branch April 22, 2026 14:54
@arkrishn94 arkrishn94 mentioned this pull request Apr 22, 2026
arkrishn94 added a commit that referenced this pull request Apr 22, 2026
Bumping to 0.50.1 to propagate changes to consumers.

Changes since previous bump: 

## What's Changed
* Add more agentic guard rails by @hildebrandmw in
#871
* Cleanup `diskann-benchmark-runner` and friends. by @hildebrandmw in
#865
* Use `--all-targets` for the no-default-features CI run. by
@hildebrandmw in #874
* Remove unused `normalizing_util.rs` from `diskann-providers` by
@Copilot in #902
* Benchmark Support for A/B Tests by @hildebrandmw in
#900
* [diskann-garnet] Bump diskann-garnet to 1.0.26 by @tiagonapoli in
#925
* Remove the `AdjacencyList` from `diskann-providers` by @hildebrandmw
in #915
* [PQ cleanup] Part 1: Move pq_scratch, quantizer_preprocess and
pq_dataset to `diskann-disk` by @arkrishn94 in
#930
* Forbid Debug in diskann-benchmark by @arrayka in
#914
* Remove DebugProvider by @JordanMaples in
#923
* [diskann-garnet] Create workflow to publish to nuget by @tiagonapoli
in #926
* Move k-means implementation from diskann-providers to diskann-disk by
@Copilot in #933
* Inline minmax distance evaluations by @arkrishn94 in
#935
* Use `rust-toolchain.toml` in CI by @hildebrandmw in
#934
* Add a globally blocking CI gate. by @hildebrandmw in
#932
* Remove `utils/math_util.rs` from `diskann-providers` by @Copilot in
#921
* Bump rand from 0.9.2 to 0.9.3 by @dependabot[bot] in
#945
* Remove OPQ and friends by @arkrishn94 in
#947
* Migrate test_flaky_consolidate from diskann_providers to diskann by
@JordanMaples in #942
* Remove GraphDataType from diskann-providers by @wuw92 in
#950
* Remove unused method extract_best_l_candidates in
NeighborPriorityQueue by @doliawu in
#951
* Add `Debug` bounds to `VectorRepr`'s distance GATs. by @hildebrandmw
in #948
* Add benchmark pipeline with Rust-native A/B validation by
@YuanyuanTian-hh in #912
* Remove unnecessary `Default` bound from `Neighbor`'s `VectorIdType` by
@doliawu in #956
* Replace `AlignedBoxWithSlice` with plain `Vec` / `Matrix` where
alignment is unused by @wuw92 in
#955
* [minmax] 8-bit benchmark by @arkrishn94 in
#959
* Add `MultiInsertStrategy` implementations for `BfTreeProvider` by
@hildebrandmw in #949
* Replace `AlignedBoxWithSlice` with `Vec` in PQScratch and disk fp
vector caches by @wuw92 in #960
* Adding unit tests for paged_search by @JordanMaples in
#962
* Remove AlignedBoxWithSlice wrapper and add alias to Poly<[T],
AlignedAllocator> by @JordanMaples in
#965
* Remove synthetic/structured data generation from diskann-providers by
@JordanMaples in #963
* added tests and some baselines for range_search by @JordanMaples in
#961

## New Contributors
* @JordanMaples made their first contribution in
#923
* @wuw92 made their first contribution in
#950
* @doliawu made their first contribution in
#951
* @YuanyuanTian-hh made their first contribution in
#912

**Full Changelog**:
v0.50.0...v0.50.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get rid of AlignedBoxWithSlice.

5 participants